Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.86%. Comparing base (50aaf4b) to head (b31acff).

@@            Coverage Diff             @@
##           master      #11      +/-   ##
==========================================
- Coverage   93.94%   93.86%   -0.09%     
==========================================
  Files          33       33              
  Lines        1800     1809       +9     
  Branches      139      141       +2     
==========================================
+ Hits         1691     1698       +7     
- Misses         70       71       +1     
- Partials       39       40       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Hi @ydejiang, thanks for the MR. I'm happy to merge this feature and perhaps make it a configurable option in the next release of riptide. A few high-level comments though:

• You're doing peak-to-peak normalisation, but we could do whitening instead, i.e. for each subint subtract the mean and divide by stddev. I honestly don't have a strong optinion on which looks best in the plots, but maybe you want to try that.
• Regardless of the normalisation scheme, it can be done without a for loop using numpy (see below).
• No need to allocate a separate array
• The atol trick is a hack and we can do better 😄

I suggest something along these lines (using whitening, but you can do the same with peak-to-peak normalisation)

means = X.mean(axis=1).reshape(-1, 1)
stddevs = X.std(axis=1).reshape(-1, 1) 
stddevs[stddevs == 0] = 1  # My replacement of the `atol` trick
# Note that this does not modify the original input array, this just shadows the X variable name with a new array
X = (X - means) / stddevs 

Hi Vincent, thank you so much for the detailed feedback!
Your example made the logic much clearer — your coding style is definitely more professional！

I tried both whitening and my original peak-to-peak normalisation on several examples.
For the diagnostic plots I tested, the simple peak-to-peak scaling actually produced slightly clearer contrast. So I’ve kept the peak-to-peak approach, but rewrote it following your suggestions — fully vectorised, no loop, and without the atol trick.

The code is much cleaner now.
Thanks again for your guidance, I really appreciate it!

@ydejiang Thank you, I always like to talk about code with anyone. Can I ask why you closed the pull request though? I was expecting just one more commit with a better version of the logic and I would have happily merged that. In any case, looking forward to the next one.

Oh, just saw you did open another PR already. All good 😄